Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CMR-10173: Update UMM-C for Web Unitification round 2 #2184

Merged
merged 14 commits into from
Nov 1, 2024
Merged

CMR-10173: Update UMM-C for Web Unitification round 2 #2184

merged 14 commits into from
Nov 1, 2024

Conversation

jmaeng72
Copy link
Contributor

@jmaeng72 jmaeng72 commented Oct 24, 2024 •
edited
Loading

Overview

What is the feature/fix?

Updating umm-c schema and translations

Adding 2 new enums values for AssociatedDOIs/Type:
IsPreviousVersionOf
IsNewVersionOf

Adding 3 new enums values for CollectionProgress:
PREPRINT, INREVIEW, and SUPERSEDED

Delete enum value for CollectionProgress:
NOT APPLICABLE

What is the Solution?

See above

What areas of the application does this impact?

Checklist

  • I have updated/added unit and int tests that prove my fix is effective or that my feature works
  • New and existing unit and int tests pass locally and remotely
  • clj-kondo has been run locally and all errors corrected
  • I have removed unnecessary/dead code and imports in files I have changed
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have cleaned up integration tests by doing one or more of the following:
    • migrated any are2 tests to are3 in files I have changed
    • de-duped, consolidated, removed dead int tests
    • transformed applicable int tests into unit tests
    • refactored to reduce number of system state resets by updating fixtures (use-fixtures :each (ingest/reset-fixture {})) to be :once instead of :each

@jmaeng72 jmaeng72 requested a review from eereiter October 24, 2024 22:42
@jmaeng72 jmaeng72 changed the title CMR-10173: Add copy of 1.18.1 as 1.18.2 CMR-10173: Update UMM-C for Web Unitification round 2 Oct 25, 2024
@jmaeng72 jmaeng72 marked this pull request as draft October 25, 2024 20:01
@jmaeng72 jmaeng72 self-assigned this Oct 25, 2024
eereiter
eereiter requested changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@eereiter eereiter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also give you sample files that you can test with on your local CMR.

@jmaeng72 jmaeng72 marked this pull request as ready for review October 30, 2024 16:37
@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 28.57143% with 25 lines in your changes missing coverage. Please review.

Project coverage is 24.42%. Comparing base (170ed1b) to head (cfb7d21).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
.../src/cmr/umm_spec/migration/version/collection.clj 13.79% 25 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (170ed1b) and HEAD (cfb7d21). Click for more details.

HEAD has 44 uploads less than BASE
Flag BASE (170ed1b) HEAD (cfb7d21)
45 1
Additional details and impacted files 
@@             Coverage Diff             @@
##           master    #2184       +/-   ##
===========================================
- Coverage   58.23%   24.42%   -33.82%     
===========================================
  Files        1054      994       -60     
  Lines       70803    67479     -3324     
  Branches     2021      955     -1066     
===========================================
- Hits        41234    16479    -24755     
- Misses      27688    50045    +22357     
+ Partials     1881      955      -926

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

eereiter
eereiter reviewed Oct 30, 2024
View reviewed changes
umm-spec-lib/resources/xml-schemas/iso19115_2/resources/Codelist/eosCodelists.xml
<gml:description>The collection supersedes the Associated DOI.</gml:description>
<gml:identifier codeSpace="gov.nasa.esdis.umm">Is_New_Version_Of</gml:identifier>
</gmx:CodeDefinition>
</gmx:codeEntry>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great Job! We already had our own code list for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eosCodelists.xml are our lists unique to our project. They differ from the standards, but that is OK, as that is what they are for.

eereiter
eereiter reviewed Oct 30, 2024
View reviewed changes
umm-spec-lib/src/cmr/umm_spec/models/umm_collection_models.clj
;; INREVIEW: Refers to datasets which are made available to support science team final review. In Review datasets are provisional and should not be used for production applications.
;; Note that if restricted access is needed, an INREVIEW dataset may also have an Access Control List applied.
;; SUPERSEDED: Refers to datasets which remain publicly available, but for which a newer version is available.
;; There is a sixth value of NOT PROVIDED that should not be used by a data provider.
;; It is currently being used as a value when a correct translation cannot be done with the
;; current valid values, or when the value is not provided by the data provider.
CollectionProgress
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you generate this? or change it by hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed it by hand

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should generate it.
If you read the README.md in the umm-spec-lib. you will see that you should run
lein generate-umm-records.

anytime you change the schemas. This will read the schemas and generate the umm defrecords.
What you did is fine, for now, but if those comments are not in the schema, the generator will remove them.

eereiter
eereiter requested changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@eereiter eereiter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, SUPERSEDED just needs to be migrated to COMPLETED going from 1.18.2 to 1.18.1

@jmaeng72 jmaeng72 merged commit 61f4bf6 into master Nov 1, 2024
6 checks passed
@jmaeng72 jmaeng72 deleted the CMR-10173 branch November 1, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TylerHeald1 TylerHeald1 TylerHeald1 approved these changes

@eereiter eereiter eereiter approved these changes

@tbs1979 tbs1979 Awaiting requested review from tbs1979

Assignees

@jmaeng72 jmaeng72

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jmaeng72 @codecov-commenter @eereiter @TylerHeald1